[ovs-dev,v2,1/3] Routing policies, add routing-policy commands in ovn-nbctl

Message ID 1542004180-202310-1-git-send-email-mary.manohar@nutanix.com
State Deferred
Headers show
Series
  • [ovs-dev,v2,1/3] Routing policies, add routing-policy commands in ovn-nbctl
Related show

Commit Message

Mary Manohar Nov. 12, 2018, 6:27 a.m.
Policy-based routing (PBR) provides a mechanism to configure permit/deny and reroute policies on the router.
Permit/deny policies are similar to OVN ACLs, but exist on the logical-router.
Reroute policies are needed for service-insertion and service-chaining.
Currently, we support only stateless policies.

To achieve this, a new table is introduced in the ingress pipeline of the Logical-router.
The new table is between the ‘IP Routing’ and the ‘ARP/ND resolution’ table.
This way, PBR can override routing decisions and provide a different next-hop.

This Series:
a. Changes in OVN NB Schema to introduce a new table in the Logical router.
b. Add commands to ovn-nbctl to add/delete/list routing policies.
c. Changes in ovn-northd to process routing-policy configurations.

This Patch:
 Add a new table 'Logical_Router_Policy' in the northbound schema.

 The table has the following columns:
 * priority: Rules with numerically higher priority take precedence over those with lower.
 * match: Uses the same expression language as the 'match' column of 'Logical_Flow' table in the OVN Southbound database.
 * action: allow/drop/reroute
 * nexthop: Nexthop IP address.

 Each row in this table represents one routing policy for a logical router.
 The 'action' column for the highest priority matching row in this table
 determines a packet's treatment. If no row matches, packets are allowed by
 default.

Signed-off-by: Mary Manohar <mary.manohar@nutanix.com>
---
 ovn/ovn-nb.ovsschema | 19 ++++++++++++++--
 ovn/ovn-nb.xml       | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 2 deletions(-)

Comments

0-day Robot Nov. 12, 2018, 6:56 a.m. | #1
Bleep bloop.  Greetings Mary Manohar, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 80 characters long (recommended limit is 79)
#59 FILE: ovn/ovn-nb.ovsschema:246:
                                            "refTable": "Logical_Router_Policy",

WARNING: Line is 93 characters long (recommended limit is 79)
#77 FILE: ovn/ovn-nb.ovsschema:318:
                                            "enum": ["set", ["allow", "drop", "reroute"]]}}},

WARNING: Line is 80 characters long (recommended limit is 79)
#123 FILE: ovn/ovn-nb.xml:1821:
        The packets that the routing policy should match, in the same expression

Lines checked: 167, Warnings: 3, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Ben Pfaff Nov. 12, 2018, 4:48 p.m. | #2
Thanks for working to make OVN better.

I've only glanced at this series.  At a glance, it looks to me like it
should be a single patch, because we usually change the schema,
implement the change in ovn-northd, and add commands to ovn-nbctl to
make it useful in a single patch.

Please add a NEWS entry and some tests of the feature in addition to the
tests of the ovn-nbctl commands.

Thanks,

Ben.

Patch

diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index f3683df..ff16985 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Northbound",
-    "version": "5.14.0",
-    "cksum": "3600467067 20513",
+    "version": "5.15.0",
+    "cksum": "3545233945 21390",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -242,6 +242,11 @@ 
                                             "refType": "strong"},
                                    "min": 0,
                                    "max": "unlimited"}},
+                "policies": {"type": {"key": {"type": "uuid",
+                                            "refTable": "Logical_Router_Policy",
+                                            "refType": "strong"},
+                                   "min": 0,
+                                   "max": "unlimited"}},
                 "enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
                 "nat": {"type": {"key": {"type": "uuid",
                                          "refTable": "NAT",
@@ -303,6 +308,16 @@ 
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
             "isRoot": false},
+        "Logical_Router_Policy": {
+            "columns": {
+                "priority": {"type": {"key": {"type": "integer",
+                                              "minInteger": 0,
+                                              "maxInteger": 32767}}},
+                "match": {"type": "string"},
+                "action": {"type": {"key": {"type": "string",
+                                            "enum": ["set", ["allow", "drop", "reroute"]]}}},
+                "nexthop": {"type": {"key": "string", "min": 0, "max": 1}}},
+            "isRoot": false},
         "NAT": {
             "columns": {
                 "external_ip": {"type": "string"},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 474b4f9..0675d39 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -1236,6 +1236,10 @@ 
       One or more static routes for the router.
     </column>
 
+    <column name="policies">
+      One or more routing policies for the router.
+    </column>
+
     <column name="enabled">
       This column is used to administratively set router state.  If this column
       is empty or is set to <code>true</code>, the router is enabled.  If this
@@ -1793,6 +1797,65 @@ 
 
   </table>
 
+  <table name="Logical_Router_Policy" title="Logical router policies">
+    <p>
+      Each row in this table represents one routing policy for a logical router
+      that points to it through its <ref column="policies"/> column.  The <ref
+      column="action"/> column for the highest-<ref column="priority"/>
+      matching row in this table determines a packet's treatment.  If no row
+      matches, packets are allowed by default. (Default-deny treatment is
+      possible: add a rule with <ref column="priority"/> 0, <code>0</code> as
+      <ref column="match"/>, and <code>drop</code> as <ref column="action"/>.)
+    </p>
+
+    <column name="priority">
+      <p>
+        The routing policy's priority.  Rules with numerically higher priority
+        take precedence over those with lower. A rule is uniquely identified
+        by the priority and match string.
+      </p>
+    </column>
+
+    <column name="match">
+      <p>
+        The packets that the routing policy should match, in the same expression
+        language used for the <ref column="match" table="Logical_Flow"
+        db="OVN_Southbound"/> column in the OVN Southbound database's
+        <ref table="Logical_Flow" db="OVN_Southbound"/> table.
+      </p>
+
+      <p>
+        By default all traffic is allowed.  When writing a more
+        restrictive policy, it is important to remember to allow flows
+        such as ARP and IPv6 neighbor discovery packets.
+      </p>
+    </column>
+
+    <column name="action">
+      <p>The action to take when the routing policy matches:</p>
+      <ul>
+        <li>
+          <code>allow</code>: Forward the packet.
+        </li>
+
+        <li>
+          <code>drop</code>: Silently drop the packet.
+        </li>
+
+        <li>
+          <code>reroute</code>: Reroute packet to nexthop
+        </li>
+      </ul>
+    </column>
+
+    <column name="nexthop">
+      <p>
+        Nexthop IP address for this route.  Nexthop IP address should be the IP
+        address of a connected router port or the IP address of a logical port.
+      </p>
+    </column>
+  </table>
+
   <table name="NAT" title="NAT rules">
     <p>
       Each record represents a NAT rule.